Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nctalk: Add option to use User ID instead of Display Name as nick #1252

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

s3lph
Copy link
Contributor

@s3lph s3lph commented Oct 1, 2020

As proposed in #1250.

Use case: The Nextcloud instance I intend to use this with gets its user accounts from an LDAP directory with nicknames as UIDs and full names as display names. However, in channels which are bridged to public IRC, I'd prefer to use nicknames rather than full names.

This change only applies to non-guest users, as guests are already handled specially and their UIDs are long hex strings (like 4247797f3cbf4fb2f4ee73995f2a2c683d32284e), which is quite unfit for display.

Copy link
Contributor

@gary-kim gary-kim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I have one suggestion if you want to implement it but your choice :)

matterbridge.toml.sample Outdated Show resolved Hide resolved
@codeclimate
Copy link

codeclimate bot commented Oct 4, 2020

Code Climate has analyzed commit f718c5c and detected 0 issues on this pull request.

View more on Code Climate.

@42wim
Copy link
Owner

42wim commented Oct 4, 2020

I've been thinking about this, I think it's better we fix this in a general way instead of adding yet another config option.

What about we add {USERID} (and {USERNAME} as an alias to {NICK}) as an option in RemoteNickFormat?
(and in future also {USERDISPLAYNAME} etc ..)

@s3lph
Copy link
Contributor Author

s3lph commented Oct 6, 2020

Come to think of it, this does sound like the more sensible approach. I just had a look at the code, the nctalk bridge already uses the same value as config.Message.UserID, so this would work.

However, there's still the issue with nctalk's guest users, their random ID still needs to be covered. Also, if more keys are introduced in the future, the RemoteNickFormat interpolation logic probably needs to be at least partially moved to the individual bridges.

@s3lph
Copy link
Contributor Author

s3lph commented Dec 3, 2020

I had some more thoughts about how to best implement this: I agree that it would be a good solution to extend the RemoteNickFormat. However, due to the special handling required for nctalk guest users, I think in this case this approach makes more sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants